fix(desktop): center compact appearance choices - #3564
Conversation
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed exact head 0af6e0371736e299b0106d45ba745691f2dc6cc6 — required test is completed / success bound to that SHA. No P0–P3.
The production change is two lines: height="100%" on the HStack inside the palette and app-icon SelectableCards. That is the correct fix rather than a nudge — the cards sit in a grid whose rows stretch to the tallest sibling, so the content box was keeping its natural height and sitting at the top of a taller row. align="center" had nothing to center against; giving the box the row's full height is what makes the existing alignment take effect.
The e2e test is the part worth calling out. It asserts cardHeight > contentHeight before asserting that the top and bottom gaps match within a pixel. That ordering is what makes it a real regression test: without the first assertion, a card whose height happens to equal its content height would satisfy the gap check trivially, and reverting height="100%" would leave the test green. Checking that the row is genuinely stretched first means the test can only pass when the condition it cares about actually exists. It also drives the real surface — sets the locale, reloads, sizes the viewport to a width where the grid actually stretches — instead of asserting against a prop.
Two small notes, neither a finding: getByRole('checkbox', { name }).locator('..') couples the test to the card's DOM nesting, so a wrapper element added inside SelectableCard would break it in a way that reads as a layout failure; and the geometry helper picks the first non-INPUT child, which is fine today but is the same coupling stated twice. Both are reasonable trade-offs for a layout assertion that has no test id to aim at.
Worth noting for whoever picks up the follow-up: these cards are still exposed with role="checkbox" for what is a single-choice control. That is the shared SelectableCard issue already tracked from #3431, not something this PR should take on.
AI-assisted review. The stretch behaviour and the test's ordering were verified against the source at this exact head. Under CONTRIBUTING.md §Review this does not count as the required independent human review — merge still needs a human committer other than the author to give LGTM and to decide.
Summary
Root cause
Astryx Grid stretches cards to the tallest item in each row, while the compact choice HStack kept its natural height. Shorter content therefore remained pinned to the card padding-top instead of sharing the extra vertical space.
Before / after
Testing
npm run build:renderer(desktop workspace)npm run typechecknpx playwright test e2e/settings.spec.ts --config e2e/playwright.config.ts --workers=1(4 passed)